-
-
Notifications
You must be signed in to change notification settings - Fork 13
Feature/integrate xp contract #603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/integrate xp contract #603
Conversation
src/aux-records/ServerConfig.ts
Outdated
| _enabled: z | ||
| .boolean() | ||
| .optional() | ||
| .default(false) | ||
| .describe( | ||
| 'Whether TigerBeetle is enabled, this allows for pre-defining and shipping default configuration.' | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LandonSiler We don't need a flag for if tiger beetle is enabled. We can simply allow the tigerBeetle key in the server config to be null/undefined to disable it.
src/aux-records/XpController.ts
Outdated
| * Get an Xp user's meta data (Xp meta associated with an auth user) | ||
| * Creates an Xp user for the auth user if one does not exist | ||
| */ | ||
| async getXpUser(id: GetXpUserById): Promise<GetXpUserResult> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LandonSiler This function should receive the ID of the currently logged in user in addition to the the ID of the user that the metadata is requested for. This is so we can validate who is allowed to access the metadata for a user and what amount of metadata they can access.
For example, we will eventually start including account balances with these metadata requests, but we don't want to return the account balance for user A to user B.
src/aux-records/XpController.ts
Outdated
| } else return result; | ||
| } | ||
| return user | ||
| ? { success: true, user } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LandonSiler Controllers should always try to be explicit about what they are returning so that we don't accidentally return more from the store than should be returned. For example, instead of:
return {
success: true,
user
};I would prefer:
return {
success: true,
user: {
id: user.id,
accountId: user.accountId,
// etc...
}
};so we can see exactly what is being returned from the controller without having to consult the store.
src/aux-records/XpController.ts
Outdated
| * Get a contract by its id | ||
| * @param id The id of the contract to get | ||
| */ | ||
| async getContractById(id: XpContract['id']): Promise<GetContractResult> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LandonSiler This function should also accept the current user ID so that we can validate whether the current user is allowed to access the given contract.
| await Promise.race([ | ||
| client.lookupAccounts([0n]), | ||
| new Promise((_, rej) => { | ||
| setTimeout(() => { | ||
| rej( | ||
| new Error( | ||
| 'Failed to connect to tigerbeetle server in time.' | ||
| ) | ||
| ); | ||
| }, 3000); | ||
| }), | ||
| ]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LandonSiler We don't need to validate the tiger beetle connection on start. If we are having issues, then the server should start but each request should fail. This helps the system recover from temporary failures and not run into issues where we have to manually restart the server executable.
4111dd0 to
7f6dd3b
Compare
…ly created or not
…nct features that were previously conflated: - Portal map layers: Add GeoJSON overlays to the map/miniMap portals - Bot map layers: Add overlays to individual bots with form: "map" Update test suite
…tions to addBotMapLayer and removeBotMapLayer in BotEvents.ts
…rer-improvements-troyce # Conflicts: # pnpm-lock.yaml
- Supports encryption, which is important for production environments
- Needs a more complete security review to ensure that proper safety controls are in place
… and SqliteFinancialStore
…find the subscription in the configuration
No description provided.